Review requested:

• @nodejs/http
• @nodejs/net

Fast-track has been requested by @mcollina. Please 👍 to approve.

@pimterry @lpinca PTAL

Fair enough, I'm on board. #64507 has indeed hit CI issues with Mac, so it doesn't look like it's the easy tiny quick fix I was hoping. I'll it finish it up on top of this afterwards to bring the fix back correctly.

Fair enough, I'm on board. #64507 has indeed hit CI issues with Mac, so it doesn't look like it's the easy tiny quick fix I was hoping. I'll it finish it up on top of this afterwards to bring the fix back correctly.

I kinda get to realize this, this maybe the problem with test code not nodejs lib code.Could you check my suggestion on #64507 thread? I think your fix is correct direction

CI: https://ci.nodejs.org/job/node-test-pull-request/74827/

@Archkon can you please send this change again so we can reapply it/rediscuss it? Sorry.

@Archkon can you please send this change again so we can reapply it/rediscuss it? Sorry.

I just think the problem with the test code where

assert.strictEqual(err.code, 'ECONNRESET');

in test-http-client-complete-response-open-request-reset.js

should be changed to like this

    assert.strictEqual(err.syscall, 'write');
    switch (err.code) {
    case 'ECONNRESET':
    case 'ECONNABORTED':
    case 'EPIPE':
      break;
    default:
      assert.fail(`Unexpected error code ${err.code}`);
  }

@Archkon can you please send this change again so we can reapply it/rediscuss it? Sorry.

I just think the problem with the test code where

assert.strictEqual(err.code, 'ECONNRESET');

in test-http-client-complete-response-open-request-reset.js

should be changed to like this

    assert.strictEqual(err.syscall, 'write');
    switch (err.code) {
    case 'ECONNRESET':
    case 'ECONNABORTED':
    case 'EPIPE':
      break;
    default:
      assert.fail(`Unexpected error code ${err.code}`);
  }

Which you could find the similar test code in
test/parallel/test-http-destroyed-socket-write2.js

    switch (er.code) {
      // This is the expected case
      case 'ECONNRESET':
        break;

      // On Windows, this sometimes manifests as ECONNABORTED
      case 'ECONNABORTED':
        break;

      // This test is timing sensitive so an EPIPE is not out of the question.
      // It should be infrequent, given the 50 ms timeout, but not impossible.
      case 'EPIPE':
        break;

      default:
        // Write to a torn down client should RESET or ABORT
        assert.fail(`Unexpected error code ${er.code}`);
    }

I just guess that the macos would differ to respond a error code.

I was wrongly be confused this when I first met the macos test failed and I change checking whether response completed to just check whether response exist.That is my bad,sorry

@pimterry @mcollina Could you check this and I have no macos device to test. Thanks!

@Archkon can you please send this change again so we can reapply it/rediscuss it? Sorry.

I just think the problem with the test code where

assert.strictEqual(err.code, 'ECONNRESET');

in test-http-client-complete-response-open-request-reset.js
should be changed to like this

    assert.strictEqual(err.syscall, 'write');
    switch (err.code) {
    case 'ECONNRESET':
    case 'ECONNABORTED':
    case 'EPIPE':
      break;
    default:
      assert.fail(`Unexpected error code ${err.code}`);
  }

Which you could find the similar test code in test/parallel/test-http-destroyed-socket-write2.js

    switch (er.code) {
      // This is the expected case
      case 'ECONNRESET':
        break;

      // On Windows, this sometimes manifests as ECONNABORTED
      case 'ECONNABORTED':
        break;

      // This test is timing sensitive so an EPIPE is not out of the question.
      // It should be infrequent, given the 50 ms timeout, but not impossible.
      case 'EPIPE':
        break;

      default:
        // Write to a torn down client should RESET or ABORT
        assert.fail(`Unexpected error code ${er.code}`);
    }

I just guess that the macos would differ to respond a error code.

I was wrongly be confused this when I first met the macos test failed and I change that to just check whether response exist.That is my bad,sorry

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.24%. Comparing base (7a11a9b) to head (fd66d5a).
⚠️ Report is 4 commits behind head on main.

@@            Coverage Diff             @@
##             main   #64511      +/-   ##
==========================================
- Coverage   90.24%   90.24%   -0.01%     
==========================================
  Files         739      739              
  Lines      241669   241655      -14     
  Branches    45543    45545       +2     
==========================================
- Hits       218087   218072      -15     
+ Misses      15150    15114      -36     
- Partials     8432     8469      +37     

CI: https://ci.nodejs.org/job/node-test-pull-request/74829/

Landed in 34c28d5